-
-
Notifications
You must be signed in to change notification settings - Fork 53
check hold label before merging #225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
I think the gmerge tool can check a hold flag got cleared before doing a merge. |
It can. But I use a quite old version of this tool, and don't plan an update |
I see. so perhaps we should add a commit/push hook to repository scripts. so it will avoid merging the stuff with unresolved holds (or not enough reviews). |
Better might be preventing merge for any hold label, not just a committer hold. |
This would be better. I do not use ghmerge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this is better than nothing.
BTW, I would like to know why people do not use ghmerge. It works very well.
review-tools/ghmerge
Outdated
rtm_set=1 | ||
if (l["name"] == "severity: urgent"): | ||
urgent_set=1 | ||
if (l["name"] == "hold: committer discussion"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please check all hold: *
labels?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping @Sashan
330d51d
to
406b427
Compare
I tested the change using this simple shell snippet:
I took pr 28691 which has a hold label
and PR 28726 which has no hold label
|
Reviewed-by: Neil Horman <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #225)
No description provided.